Skip to content
This repository was archived by the owner on Oct 8, 2025. It is now read-only.

Fix building with PHP 8.5#1647

Merged
ac000 merged 1 commit into
nginx:masterfrom
skilld-labs:1646-php85
Sep 3, 2025
Merged

Fix building with PHP 8.5#1647
ac000 merged 1 commit into
nginx:masterfrom
skilld-labs:1646-php85

Conversation

@andypost

@andypost andypost commented Aug 29, 2025

Copy link
Copy Markdown
Contributor

Proposed changes

The diable_classes is deprecated since PHP 8.5, see php/php-src@f4e2e91

Making this build conditional also require to change tests

Checklist

Before creating a PR, run through this checklist and mark each as complete:

@andypost

Copy link
Copy Markdown
Contributor Author

also I disabled following tests

		and not test_php_application_disable_classes \
		and not test_php_application_disable_classes_user \

@osokin osokin mentioned this pull request Aug 30, 2025
5 tasks
@ac000

ac000 commented Sep 1, 2025

Copy link
Copy Markdown
Member

Can we limit the #ifdeferry to the nxt_php_disable() function itself?

@andypost

andypost commented Sep 1, 2025

Copy link
Copy Markdown
Contributor Author

Yes, it could be cleaner to limit ifdef to the function only but there's no zend structure anymore so yoou need to change calling code anyway.

src/nxt_php_sapi.c:715:33: error: use of undeclared identifier 'zend_disable_class'

Another option could be deprecation of disable_classes in php mod as it will be removed in php 9 anyway so this setting will gone someday...

So ATM it's not clear which way is preferable, moreover disabled_functions also full of ifdeffery(

@ac000

ac000 commented Sep 2, 2025

Copy link
Copy Markdown
Member

It was just a thought.

I will probably squeeze this into 1.35.0 (which is likely to be the
final release for the foreseeable future...)

@ac000

ac000 commented Sep 3, 2025

Copy link
Copy Markdown
Member

@andypost

Seeing as I don't have permissions to update this branch, can you rebase
with master and make the commit message this

php: Fix building with 8.5

Link: <https://github.com/php/php-src/commit/f4e2e91d4b6d28448104500819b68edf58bd263c>
Signed-off-by: Andy Postnikov <apostnikov@gmail.com>

Thanks!

@ac000

ac000 commented Sep 3, 2025

Copy link
Copy Markdown
Member

Actually make the commit message

php: Fix building with 8.5

Closes: https://github.com/nginx/unit/issues/1646
Link: <https://github.com/php/php-src/commit/f4e2e91d4b6d28448104500819b68edf58bd263c>
Signed-off-by: Andy Postnikov <apostnikov@gmail.com>

(Note: no <> on the closes link as GitHub still doesn't recognise that)

@andypost

andypost commented Sep 3, 2025

Copy link
Copy Markdown
Contributor Author

doing

Closes: nginx#1646
Link: <php/php-src@f4e2e91>
Signed-off-by: Andy Postnikov <apostnikov@gmail.com>
@andypost

andypost commented Sep 3, 2025

Copy link
Copy Markdown
Contributor Author

ready

EDIT: somehow I can grant permission allowing maintainers to allow edits(

@ac000 ac000 merged commit de14dc7 into nginx:master Sep 3, 2025
25 checks passed
@ac000

ac000 commented Sep 3, 2025

Copy link
Copy Markdown
Member

Thanks!

@andypost andypost deleted the 1646-php85 branch September 4, 2025 00:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants